Skip to content

[SIL][PackageCMO] Allow optimizing [serialized_for_pkg] functions #73902

Merged
elsh merged 2 commits intomainfrom
elsh/pkg-cmo-inline
Jun 4, 2024
Merged

[SIL][PackageCMO] Allow optimizing [serialized_for_pkg] functions #73902
elsh merged 2 commits intomainfrom
elsh/pkg-cmo-inline

Conversation

@elsh
Copy link
Contributor

@elsh elsh commented May 24, 2024 •
edited
Loading

Allow optimizing [serialized_for_pkg] functions during SIL inlining, generic/closure specialization, and devirtualization optimization passes.

SILFunction::canBeInlinedIntoCaller now exlicitly requires a caller's SerializedKind_t arg. isAnySerialized() is added as a convenience function that checks if [serialized] or [serialized_for_pkg].

Resolves rdar://128704752

@elsh
Copy link
Contributor Author

elsh commented May 24, 2024

@swift-ci test

aschwaighofer
aschwaighofer reviewed May 24, 2024
View reviewed changes
@aschwaighofer
Copy link
Contributor

aschwaighofer commented May 24, 2024

We will also want to update the SwiftCompilerSources, they have instances of both usage of isSerialized and hasValidLinkageForFragileRef.

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyLoad.swift|284| if !function.isSerialized {
SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift|552| if parentFunction.isSerialized,                                                                                                            
SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift|554| !calleeFunction.isSerialized {
SwiftCompilerSources/Sources/SIL/Function.swift|123| public var isSerialized: Bool { bridged.isSerialized(

This code for example needs updating to take into account our canInlineInto matrix:

extension FullApplySite {                                                       
  var canInline: Bool {                                                         
    // Some checks which are implemented in C++                                 
    if !FullApplySite_canInline(bridged) {                                      
      return false                                                              
    }                                                                           
    // Cannot inline a non-inlinable function it an inlinable function.         
    if parentFunction.isSerialized,                                             
       let calleeFunction = referencedFunction,                                 
       !calleeFunction.isSerialized {                                           
      return false                                                              
    }

@elsh elsh force-pushed the elsh/pkg-cmo-inline branch from b3ef257 to d16dce6 Compare May 24, 2024 23:17
@elsh elsh requested a review from eeckstein as a code owner May 25, 2024 00:47
@elsh
Copy link
Contributor Author

elsh commented May 25, 2024

@swift-ci test

@elsh
Copy link
Contributor Author

elsh commented May 28, 2024

@swift-ci test macOS

@elsh
[SIL][PackageCMO] Allow optimizing [serialized_for_pkg] functions dur…
4ecfc96 
…ing SIL

inlining, generic/closure specialization, and devirtualization optimization passes.

SILFunction::canBeInlinedIntoCaller now exlicitly requires a caller's SerializedKind_t arg.
isAnySerialized() is added as a convenience function that checks if [serialized] or [serialized_for_pkg].

Resolves rdar://128704752
@nkcsgexi
Copy link
Contributor

nkcsgexi commented May 28, 2024

The functional part makes sense to me. Can we add tests to ensure that these optimization passes are actually active for serialized_for_pkg functions?

@aschwaighofer
Copy link
Contributor

aschwaighofer commented May 29, 2024

The added changes since my last review look good to me. We also need a change in SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift to take serializedForPackage into account:

     // Cannot inline a non-inlinable function it an inlinable function.         
    if parentFunction.isSerialized,                                             
       let calleeFunction = referencedFunction,                                 
       !calleeFunction.isSerialized {                                           
      return false                                                              
    }

@elsh
Update SILBridgedFunction APIs.
c3ded85 
Add isAnySerialized() and canBeInlinedIntoCaller(SerializedKind).
@elsh elsh force-pushed the elsh/pkg-cmo-inline branch from 5b8eaee to c3ded85 Compare May 29, 2024 22:57
elsh added a commit that referenced this pull request May 29, 2024
@elsh
Package CMO containing the following PRs.
fe44859 
#73902
#73754
#73717
#72606
#73566
#73478
#73686
#72937
#73708
#73751
#73794
@elsh elsh mentioned this pull request May 29, 2024
Merged
@elsh
Copy link
Contributor Author

elsh commented May 30, 2024

@swift-ci test

@aschwaighofer aschwaighofer mentioned this pull request Jun 4, 2024
Merged
@elsh
Copy link
Contributor Author

elsh commented Jun 4, 2024

The functional part makes sense to me. Can we add tests to ensure that these optimization passes are actually active for serialized_for_pkg functions?

More tests including this are in the upcoming PR.

@elsh elsh merged commit 6216ec6 into main Jun 4, 2024
@elsh elsh deleted the elsh/pkg-cmo-inline branch June 4, 2024 18:39
elsh added a commit that referenced this pull request Jun 5, 2024
@elsh
Package CMO containing the following PRs.
f5508cf 
#74070
#73902
#73754
#73717
#72606
#73566
#73478
#73686
#72937
#73708
#73751
#73794
elsh added a commit that referenced this pull request Jun 18, 2024
@elsh
Package CMO containing the following PRs.
c09340d 
#74531
#74468
#74416
#74070
#73902
#73754
#73717
#72606
#73566
#73478
#73686
#72937
#73708
#73751
#73794
elsh added a commit that referenced this pull request Jun 19, 2024
@elsh
Package CMO containing the following PRs.
4908327 
#74531
#74468
#74416
#74070
#73902
#73754
#73717
#72606
#73566
#73478
#73686
#72937
#73708
#73751
#73794
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aschwaighofer aschwaighofer aschwaighofer left review comments

@slavapestov slavapestov Awaiting requested review from slavapestov

@nkcsgexi nkcsgexi Awaiting requested review from nkcsgexi

@eeckstein eeckstein Awaiting requested review from eeckstein eeckstein is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@elsh @aschwaighofer @nkcsgexi